Skip to content
This repository was archived by the owner on Dec 4, 2023. It is now read-only.

Properly link to the system's libv8#161

Merged
ignisf merged 1 commit intorubyjs:masterfrom
cstrahan:link-system-install
Mar 29, 2015
Merged

Properly link to the system's libv8#161
ignisf merged 1 commit intorubyjs:masterfrom
cstrahan:link-system-install

Conversation

@cstrahan
Copy link
Copy Markdown
Contributor

@cstrahan cstrahan commented Jan 22, 2015

Before this change, the Makefile didn't tell the linker to link libv8:

LIBS = $(LIBRUBYARG_SHARED) -lpthread  -lpthread -lrt -ldl -lcrypt -lm   -lc

Now it does:

LIBS = $(LIBRUBYARG_SHARED) -lv8 -lpthread  -lpthread -lrt -ldl -lcrypt -lm   -lc

@cstrahan cstrahan force-pushed the link-system-install branch 2 times, most recently from c79378b to 233f595 Compare January 22, 2015 05:39
@cstrahan
Copy link
Copy Markdown
Contributor Author

cstrahan commented Mar 27, 2015

@cowboyd Could you take a look at this, please?

@ignisf
Copy link
Copy Markdown
Collaborator

ignisf commented Mar 27, 2015

@cstrahan, thank you for the contribution but your test is failing and you are missing a test that checks if a NotFoundError is raised

@ignisf
Copy link
Copy Markdown
Collaborator

ignisf commented Mar 27, 2015

Also, how did you notice lv8 was not specified? Could you provide steps for reproduction if there was an error?

@cstrahan
Properly link to the system's libv8
2fca693 
Before this change, the Makefile didn't tell the linker to link libv8:

    LIBS = $(LIBRUBYARG_SHARED) -lpthread  -lpthread -lrt -ldl -lcrypt -lm   -lc

Now it does:

    LIBS = $(LIBRUBYARG_SHARED) -lv8 -lpthread  -lpthread -lrt -ldl -lcrypt -lm   -lc
@cstrahan cstrahan force-pushed the link-system-install branch from 233f595 to 2fca693 Compare March 29, 2015 01:24
@cstrahan
Copy link
Copy Markdown
Contributor Author

cstrahan commented Mar 29, 2015

Also, how did you notice lv8 was not specified?

have_library(foo) adds "-l#{foo}" to the linker flags - that's what the first couple snippets were showing: the relevant lines from the Makefile before and after this patch.

Without this patch, here's the output of ldd:

$ ldd /home/cstrahan/.gem/ruby/2.1.3/gems/therubyracer-0.12.1/ext/v8/init.so
        linux-vdso.so.1 (0x00007fff27da1000)
        libruby.so.2.1 => /nix/store/1hxhmb7baqvfgqynagj7jlb764idnpyg-ruby-2.1.3-p0/lib/libruby.so.2.1 (0x00007fb7927be000)
        libpthread.so.0 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libpthread.so.0 (0x00007fb7925a1000)
        libdl.so.2 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libdl.so.2 (0x00007fb79239d000)
        libcrypt.so.1 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libcrypt.so.1 (0x00007fb792162000)
        libstdc++.so.6 => /nix/store/jrhjp66sdfv0pc95dwgdz2sly9hima23-gcc-4.8.4/lib/libstdc++.so.6 (0x00007fb791e5e000)
        libm.so.6 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libm.so.6 (0x00007fb791b5b000)
        libc.so.6 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libc.so.6 (0x00007fb7917be000)
        libgcc_s.so.1 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libgcc_s.so.1 (0x00007fb7915a7000)
        /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib64/ld-linux-x86-64.so.2 (0x00007fb792ed8000)

With this patch:

$ ldd /home/cstrahan/.gem/ruby/2.1.3/gems/therubyracer-0.12.1/ext/v8/init.so
        linux-vdso.so.1 (0x00007fff23199000)
        libruby.so.2.1 => /nix/store/1hxhmb7baqvfgqynagj7jlb764idnpyg-ruby-2.1.3-p0/lib/libruby.so.2.1 (0x00007fbd8b002000)
        libv8.so => /nix/store/ghpxjw0kbraa46sgkbfrwrhz5n8zx2x8-v8-3.16.14/lib/libv8.so (0x00007fbd8a8ec000)
        libpthread.so.0 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libpthread.so.0 (0x00007fbd8a6d0000)
        libdl.so.2 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libdl.so.2 (0x00007fbd8a4cc000)
        libcrypt.so.1 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libcrypt.so.1 (0x00007fbd8a290000)
        libstdc++.so.6 => /nix/store/jrhjp66sdfv0pc95dwgdz2sly9hima23-gcc-4.8.4/lib/libstdc++.so.6 (0x00007fbd89f8d000)
        libm.so.6 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libm.so.6 (0x00007fbd89c8a000)
        libc.so.6 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libc.so.6 (0x00007fbd898ec000)
        libgcc_s.so.1 => /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib/libgcc_s.so.1 (0x00007fbd896d6000)
        /nix/store/93zfs0zzndi7pkjkjxawlafdj8m90kg5-glibc-2.20/lib64/ld-linux-x86-64.so.2 (0x00007fbd8b71c000)

but your test is failing and you are missing a test that checks if a NotFoundError is raised

The tests now pass, and I also added the new test.

Could you provide steps for reproduction if there was an error?

I installed libv8 and therubyracer with:

$ gem build libv8.gemspec
$ gem install ./libv8-3.16.14.7.gem -- --with-system-v8=true
$ gem build therubyracer.gemspec
$ gem install therubyracer-0.12.1.gem

Is there anything else I can do to help move this along?

ignisf added a commit that referenced this pull request Mar 29, 2015
@ignisf
Merge pull request #161 from cstrahan/link-system-install
3b925dc 
Properly link to the system's libv8
@ignisf ignisf merged commit 3b925dc into rubyjs:master Mar 29, 2015
@ignisf ignisf mentioned this pull request Apr 17, 2015
Closed
@cstrahan
Copy link
Copy Markdown
Contributor Author

cstrahan commented Jun 20, 2015

@cowboyd @ignisf Would it be possible to cut a point release soon, please? As a NixOS package maintainer, I'm currently manually applying this patch in our libv8 package. It would make things easier on us if we could use 100% official source.

Thanks!

@cowboyd
Copy link
Copy Markdown
Collaborator

cowboyd commented Jun 22, 2015

Agree. Our release system involves a lot of manual grunt work to build binaries which is why we invariably end up dragging our feet on it. It's been on my list for awhile, and if it's every going to get better, we need to find a way to automate it.

It involves making an even point release, then bumping the version for binary releases on supported platforms.

I found cross-compilation images for arm7, OSX and Windows on hub.docker.com. I think if we could get a cross compilation toolchain for freebsd, then automating things would be a snap, and we could actually integrate it into CI. Thoughts @ignisf?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

status: fixed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cstrahan @ignisf @cowboyd